Skip to content

Conversation

@crass
Copy link

@crass crass commented Oct 27, 2025

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix
[ ] New feature
[X] Other, please explain: Warning message improvement

What changes did you make? (Give an overview)
Add http tracker announce url to warning messages. This change is modeled on the way that the UDP tracker does warning messages. Also, add missing space to one of the error messages.

This change is modeled on the way that the UDP tracker does warning
messages. Also, add missing space to one of the error messages.
@github-actions
Copy link

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@github-actions github-actions bot added the stale label Dec 26, 2025
@crass
Copy link
Author

crass commented Dec 31, 2025

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

Yes, it is still relevant.

@alxhotel
Copy link
Member

@crass maybe change it to onWarning

@github-actions github-actions bot removed the stale label Dec 31, 2025
@crass
Copy link
Author

crass commented Jan 1, 2026

@crass maybe change it to onWarning

Am I correct to understand you to be suggesting that I change onError to onWarning?

As stated in the PR this is how the udp tracker does it. So for consistency, in my opinion, this should only be changed if and only if the udp tracker is also changed. Are you suggesting that also?

Here's the onError function in the udp tracker which is virtually the same as the one in this PR:

function onError (err) {
cleanup()
if (self.destroyed) return
try {
// Error.message is readonly on some platforms.
if (err.message) err.message += ` (${self.announceUrl})`
} catch (ignoredErr) {}
// errors will often happen if a tracker is offline, so don't treat it as fatal
self.client.emit('warning', err)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants